Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RBAC for rss feeds #14041

Merged
merged 4 commits into from
Mar 3, 2017
Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Feb 23, 2017

First part for https://bugzilla.redhat.com/show_bug.cgi?id=1418961

Links [Optional]

@miq-bot assign @gtanzillo
@miq-bot add_label bug, core

rbac_options = {}
if user_or_group
user_or_group_key = user_or_group.kind_of?(User) ? :user : :miq_group
rbac_options = {user_or_group_key => user_or_group}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rbac_options[user_or_group_key] = user_or_group is clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure 👍, changed, thanks

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice Libor

@@ -27,7 +27,7 @@ item_class: Vm
search_method:
limit_to_time:
limit_to_count:
orderby: "created_on DESC"
orderby: "vms.created_on DESC"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@@ -9,22 +9,22 @@ def external?
resource.nil?
end

def generate(_user_or_group)
def generate(user_or_group)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice touch. this flowed through very smoothly

@kbrock kbrock assigned kbrock and unassigned gtanzillo Feb 23, 2017
@kbrock kbrock added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 23, 2017
@kbrock
Copy link
Member

kbrock commented Feb 23, 2017

kicking

@kbrock kbrock closed this Feb 23, 2017
@kbrock kbrock reopened this Feb 23, 2017
@kbrock kbrock modified the milestones: Roadmap, Sprint 55 Ending Feb 27, 2017 Feb 23, 2017
@kbrock
Copy link
Member

kbrock commented Feb 23, 2017

@lpichler I want to merge, but that failure looks like it is not going away

@@ -127,6 +127,9 @@
context "with new rss feed" do
it "init status" do
expect(MiqWidget.count).to eq(1)
puts RssFeed.class
puts RssFeed.methods.inspect
puts RssFeed.ancestors.inspect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming these are in here for debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@host2 = FactoryGirl.create(:host_microsoft, :created_on => @host1.created_on + 1.second)
end

it "#generate 2 hosts in newest_hosts rss" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this already covered by other tests?

Copy link
Contributor Author

@lpichler lpichler Feb 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just experimenting :/

I don't know what is happening here and why RssFeed is not model here https://travis-ci.org/ManageIQ/manageiq/jobs/205791463#L527 and I think that I did not make related change.

locally it is ActiveRecord

@kbrock kbrock changed the title Add RBAC for rss feeds [WIP] Add RBAC for rss feeds Feb 27, 2017
@kbrock kbrock added the wip label Feb 27, 2017
@lpichler lpichler closed this Feb 27, 2017
@lpichler lpichler reopened this Feb 27, 2017
@lpichler lpichler closed this Feb 27, 2017
@lpichler lpichler force-pushed the add_rbac_for_rss_feed branch 3 times, most recently from 5e2fe0b to 25a687e Compare March 1, 2017 17:58
to be able use it for RBAC
and process user or group as parameter for RBAC
otherwise it leads to error ambiguous column when RBAC was added.
@lpichler
Copy link
Contributor Author

lpichler commented Mar 2, 2017

@kbrock this needed to be merged first (resolution of sporadic failure)

@miq-bot
Copy link
Member

miq-bot commented Mar 2, 2017

Checked commits lpichler/manageiq@e31e658~...74351e9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 🍪

@lpichler lpichler changed the title [WIP] Add RBAC for rss feeds Add RBAC for rss feeds Mar 3, 2017
@lpichler
Copy link
Contributor Author

lpichler commented Mar 3, 2017

@miq-bot remove_label wip

This PR is ready.

@miq-bot miq-bot removed the wip label Mar 3, 2017
@kbrock kbrock modified the milestones: Sprint 56 Ending Mar 13, 2017, Roadmap Mar 3, 2017
@kbrock kbrock merged commit 6eea127 into ManageIQ:master Mar 3, 2017
@lpichler lpichler deleted the add_rbac_for_rss_feed branch March 6, 2017 08:45
simaishi pushed a commit that referenced this pull request Mar 10, 2017
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 33a86fa1bdd848d86c60599a45d180d28489e201
Author: Keenan Brock <keenan@thebrocks.net>
Date:   Fri Mar 3 14:36:55 2017 -0500

    Merge pull request #14041 from lpichler/add_rbac_for_rss_feed
    
    Add RBAC for rss feeds
    (cherry picked from commit 6eea12732de8a704a75d21040476e81597d0dccf)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1431168

@simaishi
Copy link
Contributor

Backported to Darga via #14308

agrare pushed a commit to agrare/manageiq that referenced this pull request Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants